Add option in raster plot to crop around centroids #1047
Conversation
ValentinGebhart
requested review from
emanuel-schmid,
chahank and
peanutfun
as code owners
|
Example 1: plot_intensity with floods in France Results:
Example 2: some return period maps with few centroids
|
climada/engine/impact.py
Outdated
| mask_relative_distance: float, optional | ||
| Relative distance (with respect to maximal map extent in longitude or latitude) to data | ||
| points above which plot should not display values. For instance, to only plot values | ||
| at the centroids, use mask_relative_distance=0.01. If None, the plot is not masked. | ||
| Default is None. |
There was a problem hiding this comment.
🧐 This parameter is really hard to describe. I've read it repeatedly and still need the example images to understand it.
For me it would be probably easier if it was written in a more "positive" way, something like
| mask_relative_distance: float, optional | |
| Relative distance (with respect to maximal map extent in longitude or latitude) to data | |
| points above which plot should not display values. For instance, to only plot values | |
| at the centroids, use mask_relative_distance=0.01. If None, the plot is not masked. | |
| Default is None. | |
| mask_relative_distance: float, optional | |
| Only regions are plotted that are closer to any of the data points than this distance, | |
| relative to overall plot size. For instance, to only plot values | |
| at the centroids, use mask_relative_distance=0.01. If None, the plot is not masked. | |
| Default is None. |
Though I'm not sure, whether it's better like that.
Then: don't meant to be fuzzy but the parameter name is a somewhat awkward, don't you think?
Is it not rather relative_mask_distance? Or just mask_distance? Or what about plotting_range? (as in only plot stuff within this plotting range)
There was a problem hiding this comment.
agreed, I will change the description and the parameter name to mask_distance. Thanks!
|
🙌 🎉 Only don't like the name and description too much. |
| if np.any(np.isnan(x) for x in grid_im): | ||
| if np.isnan(grid_im).any(): |
There was a problem hiding this comment.
I assume, this is the fix for the bug?
When does it occur? (How can it be reproduced?)
There was a problem hiding this comment.
Yes, the idea was that whenever grid_im has a NaN we want to include a small legend (because we plot nan values in gray). np.any(np.isnan(x) for x in grid_im) from before did not actually output a bool but a generator, such that this if condition was always True
There was a problem hiding this comment.
@ValentinGebhart Thanks! You don't happen to have an example that you could point me to?
There was a problem hiding this comment.
Sure. This would be an example where we want the legend (see PR #1038):
import numpy as np
from climada.hazard import Hazard
from climada.util import HAZ_DEMO_H5 # CLIMADA's Python file
haz_tc_fl = Hazard.from_hdf5(HAZ_DEMO_H5) # Historic tropical cyclones in Florida from 1990 to 2004
haz_tc_fl.check() # Use always the check() method to see if the hazard has been loaded correctly
centroids_mask = np.array(
[ (i + j > 10) for j in range(50) for i in range(50)]
)
haz_tc_fl.centroids = haz_tc_fl.centroids.select(sel_cen=centroids_mask)
haz_tc_fl.intensity = haz_tc_fl.intensity[:, -2434:]
return_periods, label, column_label = haz_tc_fl.local_return_period([30, 40])
from climada.util.plot import plot_from_gdf
plot_from_gdf(return_periods, colorbar_name=label, title_subplots=column_label)and here we do not want the legend but in the current develop version it would be printed as well.
haz_tc_fl.plot_intensity(event=0)
|
Thank you @emanuel-schmid for the review and the positive feedback! I would leave the default for now so people are not too confused but happy to adapt that in the future. |
As I said, not for me to judge. But I find the default plots atm are actually more confusing than a sudden change. |
I'd also agree with that but there might be other opinions about changing this plot by default @spjuhel @peanutfun @chahank ? |
|
I agree with Emanuel the plots are much clearer. (But it is not a strong opinion) |
|
I also agree that adding a default |
|
Thanks for the feedback! I changed the default to @emanuel-schmid from my side this would be ready to merge. Thanks! |
Changes proposed in this PR:
geo_im_from_arraysuch that the user can input a (relative) distance above which plot locations that are further away than this distance from any centroids are masked (ie, no values plotted). This option is then available in all function that callgeo_im_from_array, i.e.,_event_plot,hazard.plot_intensity,hazard.plot_fraction,plot_from_gdf,hazard.plot_rp_intensity,impact.plot_rp_impgeo_im_from_arrayPR Author Checklist
develop)PR Reviewer Checklist